Add Rescore functionality #1688
Conversation
|
@peermuellerxw Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
spring-projects-issues
added
the
status: waiting-for-triage
|
@peermuellerxw Thank you for signing the Contributor License Agreement! |
spring-projects-issues
added
the
status: waiting-for-triage
There was a problem hiding this comment.
Thanks for this PR, great work, but I think we should take this further. It is focused on the NativeSearchQuery, but I think we should consider the other Query implementations here as well.
Something like having a
public class RescorerQuery {
Query query; // <- our Query interface
ScoreMode scoreMode;
int windowSize;
}not using Elasticsearch classes here, but also having our own enum for the ScoreMode. This would allow to not only use NativeSearchQuery but also for example a StringQuery. In the RequestMapper this would then be mapped to the corresponding Elasticsearch classes – there are existing methods to convert a Query to a QueryBuilder.
What do you think about this change?
|
|
||
| private final QueryRescorerBuilder queryRescorerBuilder; | ||
|
|
||
| public RescorerQuery(QueryRescorerBuilder queryRescorerBuilder) { |
There was a problem hiding this comment.
this is leaking the Elasticsearch class into the API
spring-projects-issues
added
the
status: waiting-for-triage
|
Thanks for looking into my PR and for the feedback. Your suggestions do make sense and I am looking to implement them in the following days :) |
…ionality # Conflicts: # src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java # src/main/java/org/springframework/data/elasticsearch/core/query/AbstractQuery.java # src/main/java/org/springframework/data/elasticsearch/core/query/Query.java
…ata query implementations
|
I updated the PR and changed the |
spring-projects-issues
added
the
status: waiting-for-triage
| @@ -0,0 +1,94 @@ | |||
| /* | |||
| * Copyright 2020-2021 the original author or authors. | |||
There was a problem hiding this comment.
| * Copyright 2020-2021 the original author or authors. | |
| * Copyright 2021 the original author or authors. |
| public RescorerQuery(Query query) { | ||
| this.query = query; | ||
| } |
There was a problem hiding this comment.
| public RescorerQuery(Query query) { | |
| this.query = query; | |
| } | |
| public RescorerQuery(Query query) { | |
| Assert.notNull(query, "query must not be null"); | |
| this.query = query; | |
| } |
we must make sure this is never null.
| public RescorerQuery withScoreMode(ScoreMode scoreMode) { | ||
| this.scoreMode = scoreMode; | ||
| return this; | ||
| } |
There was a problem hiding this comment.
| public RescorerQuery withScoreMode(ScoreMode scoreMode) { | |
| this.scoreMode = scoreMode; | |
| return this; | |
| } | |
| public RescorerQuery withScoreMode(ScoreMode scoreMode) { | |
| Assert.notNull(scoreMode, "scoreMode must not be null"); | |
| this.scoreMode = scoreMode; | |
| return this; | |
| } |
| @@ -3122,7 +3132,67 @@ void shouldReturnHighlightFieldsInSearchHit() { | |||
| assertThat(highlightField.get(1)).contains("<em>message</em>"); | |||
| } | |||
|
|
|||
| @Test // DATAES-738 | |||
| @Test | |||
There was a problem hiding this comment.
| @Test | |
| @Test // #1686 |
| @@ -511,6 +522,139 @@ private String requestToString(ToXContent request) throws IOException { | |||
| return XContentHelper.toXContent(request, XContentType.JSON, true).utf8ToString(); | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
| @Test | |
| @Test // #1686 |
|
|
||
| private QueryRescorerBuilder getQueryRescorerBuilder(RescorerQuery rescorerQuery) { | ||
|
|
||
| QueryRescorerBuilder builder = new QueryRescorerBuilder(Objects.requireNonNull(getQuery(rescorerQuery.getQuery()))); |
There was a problem hiding this comment.
Instead of a NullPointerException we should throw an IllegalArgumentException here
Closes #1686